fix(pr-pipeline): skip a lane's key leg while its key is out of credit - #12617

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

claude-security-review reviewed 17afcda6d6b3b33624f5d2bc30f7089f329da3b5: read 6 of 6 changed file(s); 0 finding(s), 0 as inline comments.

Security review: no security issues found

critical-findings: 0

I read every hunk of all 6 changed files. The two workflow files are trusted paths, so I read their whole change and what it gates.

What I checked

• New credit-gate jobs (check-intent-credit, review-handoffs-credit):
  • Each job's only permission is actions: read. It references no secret, does no checkout and starts no model.
  • The gate action is pinned to a full SHA in the owner's own ci-workflows repository: check-key-credit@2476d5dd…, at pr-verify.yml#L871.
  • origin-run and github.run_id appear only in step names. Every run: body is a fixed echo, so no expression reaches a shell.
• What skip-key changes. It only picks which credential leg judges. Even if it were set wrongly, the work just moves to the subscription leg, and that leg still runs all of its own gates:
  • In pr-verify, check-intent-subscription still runs the kill switch, resolve-pr.mjs and check-trusted-trigger against the trusted-actor list (pr-verify.yml#L2089-L2124).
  • When the key job is skipped, CARRIED is empty, so resolve-pr resolves the pull request itself; the comment at L2100 says it takes the carried reads only when they hold.
  • check-intent-subscription keeps its trigger gate (same-repository pull_request run on main). The fallback job now requires needs.check-intent-subscription.result == 'success' explicitly.
• Changed if: conditions. The key jobs' new !cancelled() plus an explicit needs.trigger.result == 'success' keep the same trigger, fork and ref conditions. A failed probe runs the key leg as before, so nothing new gets through.
• New route in pr-review-main. review-handoffs keeps permissions: {}. The main-sha fallback comes from the trigger job's checkout of the default branch. The route's jq still accepts only the reasons credit-stop auth no-key and only a base matching ^[0-9a-f]{40}$ (pr-review-main.yml#L1264-L1271). review-subscription keeps its own trigger gate and read-only permissions.
• record job. It now also runs when the key leg was skipped but the subscription leg ran. It still records only what write-verdict.mjs checked from this run's own artifacts.
• Trust controls. Nothing widens:
  • who passes the trusted-trigger check;
  • lane token scope;
  • the kill switch (every model leg still checks it);
  • merge holds (no hold file was touched).
• Instruction surfaces and plugin manifests. None changed.
• Test files. gate-expression.mjs and the three test files only run in tests and have no security surface.

What it spends: the subscription leg now does the work while the key is out of credit, which spends subscription usage. Each run also adds one short probe job on a standard ubuntu-24.04 runner. No paid plan or billing setting changes.

The pull request description contains no embedded instructions to this lane.

claude-review reviewed 17afcda6d6b3b33624f5d2bc30f7089f329da3b5: read 6 of 6 changed file(s); 0 finding(s), 0 as inline comments.

Code review: no findings

I read every hunk of all 6 changed files in .claude-lane/pr.diff. I also read the job graphs of pr-verify.yml and pr-review-main.yml at head to check every job downstream of a key leg the gate skips.

What changes. A new job in each workflow (check-intent-credit and review-handoffs-credit) runs ci-workflows check-key-credit. While the key's last credit stop is inside the window, that job skips the key job and hands the work to the subscription leg as a credit-stop. A shared test helper, .github/scripts/lib/gate-expression.mjs, evaluates job if: expressions for the tests.

What I checked, with nothing to report:

• Skipped key job in pr-verify.yml. Every job downstream now tolerates a skipped check-intent:
  • check-intent-subscription adds !cancelled() and checks needs.check-intent.result == 'success' explicitly.
  • check-intent-fallback adds !cancelled() and checks the subscription job's success explicitly.
  • wake-pr-merge already used !cancelled().
  • An empty needs.check-intent.outputs.reads makes resolve-pr.mjs do its own reads ("taken only when they hold").
• Skipped key job in pr-review-main.yml.
  • review checks needs.trigger.result == 'success' in place of the implicit success() it had before.
  • review-subscription already used !cancelled() together with needs.review-handoffs.result == 'success'.
  • The new condition on record runs it only when a key or subscription leg could have reviewed.
  • review-handoffs hands lanes on only when this attempt's trigger job would have started the key leg. The trigger's files step runs only when at least one lane is enabled, so its review == 'true' output means at least one lane is on. That keeps the route's matrix from coming out empty.
  • CODE_BASE/SECURITY_BASE fall back to the new main-sha output, which a step writes right after the trigger job's checkout.
• Whether a probe failure blocks review. It does not. The probe step has continue-on-error, and the key job's if starts with !cancelled(), so a failed or skipped probe leaves skip-key empty and the key runs as before.
• Where the credit records are written. Each key leg records its own stop only after a credit stop (always() / !cancelled() plus the handoff-reason checks). A lane the gate handed on writes no new record in review-handoffs, so the window still counts from the original stop.
• gate-expression.mjs. It gives && higher precedence than || and consumes every token, and it asserts that the whole gate was tokenized and parsed. It matches GitHub's semantics for every shape these workflows use.

Not verified (no shell in this lane). The test, zizmor and actionlint results in the PR body are the author's claims; I did not run them.

Unverified observation, not counted as a finding. The PR body says the probe runs on every run so the record stays inside the gate's three-run read. In pr-review-main.yml, review-handoffs-credit has an if that still requires workflow_run.conclusion == 'success' and a same-repository pull_request run. If check-key-credit counts runs without the job toward its read, a few such runs in a row could drop the record. The cost would be one extra key leg that fails and records again. Whether this happens depends on gate.cjs in ci-workflows, which I could not read.

Criteria sources. The PR only references #12526 and closes no issue, so there are no acceptance criteria to check. No .claude/rules/ file's paths: matches a changed file. No plugin manifest changed. There are no prior findings, so the round cap does not apply. I found no embedded instructions in the PR context.

Spend (not a hold). The PR adds one small job per run in each workflow, with no model and no secret, on ubuntu-24.04. It cuts key-leg API calls that would fail on no credit; the work moves to subscription legs that already run today.

Post-merge mutation testing

• Range: 304c2ae..0fa4041
• Merged pull request: #12617
• Outcome: reported

Killed 5, survived 2, timeout 0, no coverage 0, invalid 0, not run 0.
Covered-code score 71%; mutation score 71%.

Surviving mutants

The run's summary has the full report.